feat(metrics): implement metric reader metrics#4970
feat(metrics): implement metric reader metrics#4970anuraaga wants to merge 10 commits intoopen-telemetry:mainfrom
Conversation
…y-python into metrics-reader-metrics
|
Sent open-telemetry/opentelemetry-python-contrib#4332 to make genai test only read its scope |
anuraaga
force-pushed
the
metrics-reader-metrics
branch
from
120c3db to
3c70e2a
Compare
| raise Exception(f"Invalid instrument class found {typ}") | ||
|
|
||
| self._otel_component_type = ( | ||
| otel_component_type or type(self).__qualname__ |
There was a problem hiding this comment.
I believe there are known values for otel.component.type that must be used if applied.
e.g. periodic_metric_reader
In addition, perhaps we can also be consistent and apply underscore case for other defaults like memory_metric_reader.
| ) -> None: | ||
| meter = meter_provider.get_meter("opentelemetry-sdk") | ||
|
|
||
| count = _component_counter[component_type] |
There was a problem hiding this comment.
Do we need a lock here or would that be overkill since in practice MetricReaders are usually instantiated on application start?
| ] | ||
| | None = None, | ||
| *, | ||
| otel_component_type: str | None = None, |
There was a problem hiding this comment.
| otel_component_type: str | None = None, | |
| otel_component_type: OtelComponentTypeValues | None = None, |
| metric_reader._set_collect_callback( | ||
| self._measurement_consumer.collect | ||
| ) | ||
| metric_reader._set_meter_provider(self) |
There was a problem hiding this comment.
This seems to be introducing a new pattern of setting the meter provider instead of explicitly passing in one and then defaulting to the global. See tracer metrics and logger metrics. Should we do something similar to be consistent?
| metrics = self._collect(self, timeout_millis=timeout_millis) | ||
| start_time = perf_counter() | ||
| try: | ||
| metrics = self._collect(self, timeout_millis=timeout_millis) |
There was a problem hiding this comment.
Looks like the spec indicates that we should capture error.type as well on a failed collection.
|
Thanks for your contribution! Could you update the title + description to include that this PR covers specifically implementing the |
3 participants
Description
I am helping to implement SDK internal metrics
https://opentelemetry.io/docs/specs/semconv/otel/sdk-metrics/
This implements the metric for metric reader collections.
Similar PR in Java: open-telemetry/opentelemetry-java#8038
Similar PR in JS: open-telemetry/opentelemetry-js#6449
/cc @xrmx to help with review
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Does This PR Require a Contrib Repo Change?
Checklist: